Skip to content

Reckless: dont crash the plugin #8455

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
Aug 18, 2025
Merged

Reckless: dont crash the plugin #8455

merged 3 commits into from
Aug 18, 2025

Conversation

@endothermicdev
Copy link
Collaborator

@endothermicdev endothermicdev commented Aug 14, 2025

Checklist

Before submitting the PR, ensure the following tasks are completed. If an item is not applicable to your PR, please mark it as checked:

  • The changelog has been updated in the relevant commit(s) according to the guidelines.
  • Tests have been added or modified to reflect the changes.
  • Documentation has been reviewed and updated as needed.
  • Related issues have been listed and linked, including any that this PR closes.

The legacy uv installer was extremely verbose and the flood of output caused a crash in the reckless-rpc plugin. This gives the plugin time to react, but also reduces the verbosity as most of it was dependency resolution from requirements.txt which was generally superfluous.

@endothermicdev endothermicdev added this to the v25.09 milestone Aug 14, 2025
@endothermicdev
Copy link
Collaborator Author

endothermicdev commented Aug 14, 2025 •
edited
Loading

fixes: #8444

@ShahanaFarooqui ShahanaFarooqui linked an issue Aug 14, 2025 that may be closed by this pull request
reckless-rpc installation fails but direct installation via reckless works #8444
Closed
@ShahanaFarooqui
Copy link
Collaborator

ShahanaFarooqui commented Aug 14, 2025

Consolidate plugin installation passes after 2-3 tries but summary still keeps failing with error:

...
"INFO: dependencies installed successfully",
"INFO: plugin installed: <home>/l1/reckless/summary",
"DEBUG: activating summary",
"ERROR: reckless: summary failed to start!",
"ERROR: CLIError(-32602 <home>/l1/reckless/summary/summary.py: Invalid argument)",
"ERROR: dynamic activation failed: summary not found in reckless directory"

rustyrussell
rustyrussell requested changes Aug 15, 2025
View reviewed changes
tests/test_reckless.py Outdated
header = line


@unittest.skipIf(VALGRIND, "virtual environment triggers memleak detection")
Copy link
Contributor

@rustyrussell rustyrussell Aug 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better is #8457

@endothermicdev
Copy link
Collaborator Author

endothermicdev commented Aug 15, 2025

ERROR: CLIError(-32602 /l1/reckless/summary/summary.py: Invalid argument)

This is lightning-cli being called and rejecting the lightning-cli plugin start <location> command, due to an invalid argument. I wasn't able to uncover much as to why it's being rejected - the location was accurate. And retrying, it was eventually accepted. But I would have expected a <location> is not executable: No such file or directory error, not a rejection of the command format.

ShahanaFarooqui
ShahanaFarooqui approved these changes Aug 15, 2025
View reviewed changes
Copy link
Collaborator

@ShahanaFarooqui ShahanaFarooqui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 98b524b

@ShahanaFarooqui
Copy link
Collaborator

ShahanaFarooqui commented Aug 15, 2025

This is lightning-cli being called and rejecting the lightning-cli plugin start <location> command, due to an invalid argument. I wasn't able to uncover much as to why it's being rejected - the location was accurate. And retrying, it was eventually accepted. But I would have expected a <location> is not executable: No such file or directory error, not a rejection of the command format.

It is strange that when reckless plugin calls the same lightning-cli command

[2025-08-15 10:37:14,830] DEBUG: calling ['lightning-cli', '--network=regtest', '--lightning-dir=/home/shahana/workspace/dummy-networks/regtest-network/l1', 'plugin', 'start', '/home/shahana/workspace/dummy-networks/regtest-network/l1/reckless/summary/summary.py']

It successfully starts the plugin and fixes subsequent reckless-rpc calls in the process too.

@rustyrussell
Copy link
Contributor

rustyrussell commented Aug 17, 2025

Removed first commit, rebased.

@rustyrussell rustyrussell force-pushed the reckless-dont-crash-plugin branch from 98b524b to 7cf95b0 Compare August 17, 2025 00:19
@endothermicdev @rustyrussell
reckless: store source locations as correct type
e078332
@endothermicdev @rustyrussell
reckless: reduce uv verbosity and avoid flooding output
e67dd18 
This was overloading the reckless-rpc plugin input when outputting
json all in one shot. The verbosity was mostly dependency resolution
which wasn't all that helpful so call uv pip install as normal.

Changelog-None: bug introduced this release.
@endothermicdev @rustyrussell
reckless-rpc: increase initial buffer size
906bc7a 
Reckless uv installations typically produce 3KB+ of output.
@rustyrussell rustyrussell force-pushed the reckless-dont-crash-plugin branch from 7cf95b0 to 906bc7a Compare August 18, 2025 01:52
@rustyrussell rustyrussell self-requested a review August 18, 2025 02:27
@rustyrussell rustyrussell merged commit 1d513c2 into ElementsProject:master Aug 18, 2025
73 of 78 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rustyrussell rustyrussell rustyrussell approved these changes

@ShahanaFarooqui ShahanaFarooqui ShahanaFarooqui approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v25.09

Development

Successfully merging this pull request may close these issues.

reckless-rpc installation fails but direct installation via reckless works

3 participants

@endothermicdev @ShahanaFarooqui @rustyrussell